-
Notifications
You must be signed in to change notification settings - Fork 205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
handle $aborted with a bit more logic #97
Conversation
} | ||
if (($aborted||'') eq 'obsolete') { | ||
$result = $overall if !$aborted && $overall; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if I understand right, suppose a case here: aborted due to cancel, then aborted=cancel, result=cancel, overall is none, in the end, worker will call api_call('post', 'jobs/'.$job->{'id'}.'/set_done', {result => cancel}); I think, but I don't think we have "cancel" in our job_results.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: I just moved that line.
we have JobState cancelled. So the right thing to do here is result='incomplete', state='cancelled' - but it looks like the job is first set to cancelled by openQA and then the worker is told. So actually the worker doesn't need to do anything, so it doesn't really harm in the light of things if the worker sets an invalid result (and get a 500 as return).
I'll try to clean this up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, not really matter with this line, I just not sure where is the best line to add the comment, so just pick this one.
b2470e4
to
f5a8cb8
Compare
please give it another review - I will test it now :) |
it's still buggy - not my changes but the whole thing :) |
hmm...the changes LGTM then I don't understand where is buggy :) |
f5a8cb8
to
ed2fb36
Compare
qemu wans't properly dead when it started a new job - this explains the issues I had in https://progress.opensuse.org/issues/5344 |
LGTM |
all ends go into stop_job and kill_worker now and there it waits.
ed2fb36
to
187a169
Compare
Ok with me. Nothing going against my plan :) |
handle $aborted with a bit more logic
happy rebase then |
trying to preserve the functionality though